Skip to content

[thezerohour] iP#177

Open
thezerohour wants to merge 25 commits intonus-cs2113-AY2425S2:masterfrom
thezerohour:master
Open

[thezerohour] iP#177
thezerohour wants to merge 25 commits intonus-cs2113-AY2425S2:masterfrom
thezerohour:master

Conversation

@thezerohour
Copy link
Copy Markdown

Duke renamed to Pixy, implemented up to Level-3, A-CodingStandard.

Copy link
Copy Markdown

@vihaan27 vihaan27 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, looks good and follows convention generally. Be careful about if-else statements.

System.out.println((i + 1) + ".[" + tasks[i].getStatusIcon() + "] " + tasks[i].description);
}
}
else if (command.contains("unmark")) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

else-if statements should be on previous line (same line as if-block closing bracket)

}
System.out.println("Bye. Hope to see you again soon!");
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, well named variables and followed conventions

System.out.println("Nice! I've marked this task as done:");
System.out.println((taskIndex) + ".[" + tasks[taskIndex - 1].getStatusIcon() + "] " + tasks[taskIndex - 1].description);
}
else {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

else statement should also be on previous line

public void setDone(boolean done) {
isDone = done;
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

Copy link
Copy Markdown

@QuyDatNguyen QuyDatNguyen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, good work. However, the layout of the code can be further improved for better readability.

public class Pixy {
public static void main(String[] args) {
Scanner in = new Scanner(System.in);
Task[] tasks = new Task[100];
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should avoid magic number 100

return (isDone ? "X" : " ");
}

public void setDone(boolean done) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The naming of variable "done" should be more clear/ more "boolean"

System.out.println("Hello! I'm Pixy");
System.out.println("What can I do for you?");
String command = in.nextLine();
int tasksCounter = 0;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The lay out can be further improved by breaking the codes into different sections using spaces/ empty lines

}
command = in.nextLine();
}
System.out.println("Bye. Hope to see you again soon!");
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is actually a bit of ambiguity on which command does this line responding too (I could not keep track of which command at this point)

Copy link
Copy Markdown

@aditichadha1310 aditichadha1310 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

overall, good work!


@Override
public String toString() {
return "[D]" + super.toString() + " (from: " + from + ", to: " + to + ")";
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid usage of magic strings as much as possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants